-
Notifications
You must be signed in to change notification settings - Fork 456
CDRIVER-4689 Implement OIDC machine callback #2147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CDRIVER-4689 Implement OIDC machine callback #2147
Conversation
cc5f06a to
c57816d
Compare
Co-authored-by: mdb-ad <[email protected]>
> WARNING: task group 'test-oidc-task-group' has a teardown task timeout of 3600 seconds, which exceeds the maximum of 180 seconds
|
|
||
| #include <bson/error.h> | ||
|
|
||
| // mongoc_oidc_append_speculative_auth adds speculative auth. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Nitpick) I don't think this comment is adding any useful context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Removed.
| mongoc_cluster_run_command_monitored(mongoc_cluster_t *cluster, mongoc_cmd_t *cmd, bson_t *reply, bson_error_t *error) | ||
| { | ||
| bool ok = run_command_monitored(cluster, cmd, reply, error); | ||
| if (!ok) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you could flatten this a bit, e.g.:
if (run_command_monitored(cluster, cmd, reply, error)) {
return true;
}
// rest of the function bodyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Updated.
|
|
||
| if (strcasecmp(mechanism, "MONGODB-OIDC") == 0) { | ||
| // Expect successful reply to include `done: true`: | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superfluous compound statement since the braces after the if already form a compound statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Summary
Implement Machine Authentication Flow for OIDC auth. This enables using the
MONGODB-OIDCauth mechanism with a user-supplied callback giving the access token. Integrating other identity providers is planned in future work: Azure (CDRIVER-4548) and GCP (CDRIVER-4611).mongoc_client_set_oidc_callbackandmongoc_client_pool_set_oidc_callback.binaryoperation to the BSON DSL to help construct the OIDC commands.Tested with https://spruce.mongodb.com/version/68e90bef5727ed0007c8e376
Background & Motivation
mongoc_cluster_run_command_monitoredchecks for aReauthenticationRequirederror from the server and retries once. Ths same change is not made inmongoc_cluster_run_command_partsormongoc_cluster_run_command_private(used for auth commands, and not expected to need retry).mongoc_client(_pool)_set_oidc_callbackreturns a bool and logs on error for consistency with other recently added setters likemongoc_client(_pool)_set_structured_log_opts.Testing
Evergreen tasks are added following Scripts for OIDC testing > Evergreen Testing.
Tests can be run locally by starting the OIDC-enabled server:
Then running: